Skip to content

Set vertical bounds to prevent accesses outside of halo region#375

Merged
mark-petersen merged 2 commits intoE3SM-Project:developfrom
mwarusz:omega/minmax-halo
Apr 6, 2026
Merged

Set vertical bounds to prevent accesses outside of halo region#375
mark-petersen merged 2 commits intoE3SM-Project:developfrom
mwarusz:omega/minmax-halo

Conversation

@mwarusz
Copy link
Copy Markdown
Member

@mwarusz mwarusz commented Mar 24, 2026 •
edited
Loading

This PR changes how the [MinLayerEdgeBot, MaxLayerEdgeTop] and [MinLayerVertexBot, MaxLayerVertexTop] index ranges are defined for the outermost edges and vertices of the halo layer. In this PR they are set to be invalid (empty) ranges to prevent accesses outside the halo.

Checklist

  • Linting
  • Building
    • CMake build does not produce any new warnings from changes in this PR
  • Testing
    • Add a comment to the PR titled Testing with the following:
      • Which machines CTest unit tests
        have been run on and indicate that are all passing.
      • The Polaris omega_pr test suite
        has passed, using the Polaris e3sm_submodules/Omega baseline
      • Document machine(s), compiler(s), and the build path(s) used for -p for both the baseline (Polaris e3sm_submodules/Omega) and the PR build
      • Indicate "All tests passed" or document failing tests

@mwarusz
Copy link
Copy Markdown
Member Author

mwarusz commented Mar 25, 2026 •
edited
Loading

Testing

CTest unit tests:

Machine: frontier
Compiler: craygnu-mphipcc
Build type: Release
Result: All 39 tests passed
Log: /lustre/orion/cli115/scratch/mwaruszewski/omega-pr-testing/minmax-halo/build-craygnu-mphipcc/Testing/Temporary/LastTest.log

Polaris omega_pr suite:

Machine: aurora
Compiler: oneapi-ifx
Baseline workdir: /lus/flare/projects/E3SM_Dec/mwaruszewski/omega-pr-testing/minmax-halo/baseline-ifx
Baseline build:  /lus/flare/projects/E3SM_Dec/mwaruszewski/omega-pr-testing/minmax-halo/baseline-ifx/build
PR build: /lus/flare/projects/E3SM_Dec/mwaruszewski/omega-pr-testing/minmax-halo/build-ifx
PR workdir: /lus/flare/projects/E3SM_Dec/mwaruszewski/omega-pr-testing/minmax-halo/pr-ifx
Build type: <Release>
Log: /lus/flare/projects/E3SM_Dec/mwaruszewski/omega-pr-testing/minmax-halo/pr-ifx/polaris_omega_pr.o8399514
Result: All tests passed

@mwarusz mwarusz requested review from brian-oneill and sbrus89 March 25, 2026 16:11
@mwarusz mwarusz mentioned this pull request Mar 25, 2026
Open
sbrus89
sbrus89 approved these changes Apr 1, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @mwarusz, thanks for making this fix. Passes CTests on pm-cpu with debug build.

@mwarusz mwarusz mentioned this pull request Apr 3, 2026
Merged
19 tasks
brian-oneill
brian-oneill requested changes Apr 5, 2026
View reviewed changes
Copy link
Copy Markdown

@brian-oneill brian-oneill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks mostly good, but a couple bugs. After fixing them, I confirmed identical behavior between MPAS-O and Omega.

@mwarusz mwarusz force-pushed the omega/minmax-halo branch from aa4c3d9 to 5142fc0 Compare April 6, 2026 14:57
@mwarusz
Copy link
Copy Markdown
Member Author

mwarusz commented Apr 6, 2026

Those were some serious bugs. Clearly, I should have checked that this PR actually fixes #375. Thanks for catching them @brian-oneill !

@brian-oneill
Copy link
Copy Markdown

brian-oneill commented Apr 6, 2026

Oops, also need to add OMEGA_SCOPE(LocNCellsAll, NCellsAll); to minMaxLayerEdge and minMaxLayerVertex

@mwarusz @brian-oneill
Use NCellsAll to look for invalid cells
9f58f44 
Co-authored-by: Brian O'Neill <boneill@lanl.gov>
@mwarusz mwarusz force-pushed the omega/minmax-halo branch from 5142fc0 to 9f58f44 Compare April 6, 2026 15:34
@mwarusz
Copy link
Copy Markdown
Member Author

mwarusz commented Apr 6, 2026

Done.

@mark-petersen mark-petersen self-assigned this Apr 6, 2026
@mark-petersen
Copy link
Copy Markdown
Collaborator

mark-petersen commented Apr 6, 2026

Merged locally to develop. Passes PM gnu cTests on CPU and GPU.

@mark-petersen mark-petersen merged commit 1e1a52d into E3SM-Project:develop Apr 6, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbrus89 sbrus89 sbrus89 approved these changes

@brian-oneill brian-oneill brian-oneill approved these changes

Assignees

@mark-petersen mark-petersen

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mwarusz @brian-oneill @mark-petersen @sbrus89